fix: keep Redis SUBSCRIBE alive by polling get_message instead of listen()#96
Open
tjluyao wants to merge 1 commit into
Open
fix: keep Redis SUBSCRIBE alive by polling get_message instead of listen()#96tjluyao wants to merge 1 commit into
tjluyao wants to merge 1 commit into
Conversation
The dispatch/command/response listeners consume via iter_pubsub_messages, which
looped over pubsub.listen(). listen() parks inside a single parse_response for
the whole idle lifetime of a subscription, so redis-py's health_check_interval
(set alongside socket_keepalive) fires at most once and no application traffic
flows on a quiet channel. An idle proxy/LB then culls the SUBSCRIBE connection
(TCP keepalive ACKs don't reset an L7 inactivity timer) and the node silently
goes dispatch-dead ('No subscribers on tasks channel'), which no amount of
socket_keepalive config could prevent.
Consume with get_message(timeout=1.0) instead: each call runs check_health(),
emitting the periodic PING that keeps the connection warm and the subscription
registered. Fixes the chronic ~1h-after-restart fleet dispatch death.
Signed-off-by: Yao Lu <ylu@yao.lu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause (chronic fleet dispatch death)
Field nodes go dispatch-dead ~1 h after every (re)start: registered + heartbeating, but
PUBSUB NUMSUB node:<id>:dispatch= 0, so the host logsNo subscribers on tasks channeland tasks failmax_attempts_exceeded. Restarting a box fixes it for ~1 h, then it recurs. A prior change addedsocket_keepalive+socket_keepalive_options(TCP_KEEPIDLE 60/…) +health_check_interval=30on the client, but the deaths continued.The reason: all three listeners (dispatch, cmds, node responses) consume through
iter_pubsub_messages, which looped overpubsub.listen().listen()parks inside a singleparse_response(block=True)for the entire idle lifetime of the subscription — redis-py only runscheck_health()on entry toparse_response, so on a quiet channel the health-check PING fires at most once and then never again. With no application-level traffic, the LB's idletimeout_serverculls the SUBSCRIBE connection (TCP keepalive ACKs are L4 and don't reset HAProxy's L7 inactivity timer).health_check_intervalwas set but never driven.Fix
Consume with
get_message(timeout=1.0)in a loop instead oflisten(). Every iteration callscheck_health(), which emits the periodic PING (everyhealth_check_interval=30 s) that keeps the connection warm and the subscription registered — well under any idle timeout. One-line-of-behavior change in the shared helper, so it fixes the dispatch listener, command listener, and node-response listener at once.Tests
tests/server/test_iter_pubsub_messages.py: asserts it polls viaget_message(timeout)(neverlisten()), idleNoneticks don't end iteration, bad-JSON is skipped, andConnectionErrorstops cleanly.Deploy
Server-only change → new
flowmesh_serverimage, rolled to the fleet (same recipe as the rc.3 rollout). Complements #95 (dispatcher no longer mis-reports undeliverable dispatch) and deploy_infra #8 (watchdog force-restart as a safety net). This PR removes the need for either as a recovery mechanism by preventing the cull in the first place.